-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HUDI-6508] Fix compile errors with JDK11 #9300
Conversation
@@ -43,16 +43,6 @@ | |||
<plugin> | |||
<groupId>net.alchim31.maven</groupId> | |||
<artifactId>scala-maven-plugin</artifactId> | |||
<executions> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will build failue at JDK11, see davidB/scala-maven-plugin#234
Note: this module does not need the scala plugin at all, so just remove it, like #8336
Hi @Zouxxyy , thanks for posting this PR! I think we've had an existing JIRA to track this issue: https://issues.apache.org/jira/browse/HUDI-6508 Could you use this to track the compile time support instead please? |
@CTTY Yes, done. In addition, do you understand why JDK11 compilation occurs in the current CI (Some PRs failed for this, and it seems to appear randomly) before this PR ~ |
Hmm I've seen this before when we were using hudi/.github/workflows/bot.yml Line 32 in cb8ff26
But I've not seen this error again since that. It looks like a random bug with GitHub CI |
@CTTY Ohh, the error seems still occurs after update to checkout@v3. Anyway, let's fix the compilation errors first |
@@ -158,8 +155,7 @@ class TestMetadataRecordIndex extends HoodieSparkClientTestBase { | |||
} | |||
|
|||
private def getWriteConfig(hudiOpts: Map[String, String]): HoodieWriteConfig = { | |||
val props = new Properties() | |||
props.putAll(hudiOpts.asJava) | |||
val props = TypedProperties.fromMap(hudiOpts.asJava) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #8511
@@ -126,10 +126,10 @@ jobs: | |||
|
|||
steps: | |||
- uses: actions/checkout@v3 | |||
- name: Set up JDK 8 | |||
- name: Set up JDK 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when we have a java11 pipline, move to there
Let's stick to Java 8 compilation for now to avoid any issue for the upcoming 0.14.0 release. I think @CTTY is also working on making Hudi compilation working on Java 17. I also have a WIP branch to make Java 17 build master...yihua:hudi:java17-build Shall we merge the effort here? |
This PR is just build with java11 at java17 test pipeline, and without any package upgrade or code logic change, I think it's harmless (CI is pass and I also tested locally with my jdk11) when we have a java11 test pipline, move it to there. I fixed it mainly because some PRs will be blocked currently. e.g. see
Yeah, ok, looks like the changes are basically the same, except for the upgrade of scala-maven-plugin.version and scala version |
@yihua @CTTY @danny0405 Hi, can you help with a review~ Many PRs have compilation errors now |
@Zouxxyy : Can you fix the merge conflicts ? |
dbad787
to
75f2315
Compare
done |
Change Logs
Currently, JDK11 compilation will occur in the current CI (Some PRs failed for this, and it seems to appear randomly) . In order not to block merge, and support JDK11 in the future:
Impact
Fix compile errors with JDK11
Risk level (write none, low medium or high below)
none
Documentation Update
none
Contributor's checklist